test: Add hack to avoid losing focus on #containers-filter #1356

martinpitt · 2023-07-14T15:41:23Z

For some reason, .focus() sometimes does not take effect, and the element immediately loses the focus again. This causes set_input_text() to select the whole page text instead of the input element, and failing to set the intended value.

This fixes this common flake, another example. So far this breaks pretty much every PR which runs tests three times (affected).

I realize this is a rather lame workaround, not a true fix. I investigated that in #1324 (comment) and attempted a better fix, but it didn't work, and I gave up after some hours ("pick your battles"). If someone is interested in tracking this down, please do!

I stress-tested this in #1324, and it passed 15 times in all operating systems at the first try (I amplified the test)

marusak · 2023-07-17T14:11:07Z

First thing that came to mind was #439 (we have workaround for this in tests)

The workaround is indeed very hacky and very much focus on one specific place but I would think that if focus is lost it will happen on more places and I would either prefer more generic hack or at least knowing what steals the focus since we don;t have this problem in other plugins. You mentioned in #1324 that there are status updates - have you checked if these actually are linked to losing of focus?

martinpitt · 2023-07-17T15:28:44Z

Yeah, fair enough. I already tried a generic fix in cockpit-project/cockpit#19078 , and it was a spectacular failure -- both broken and not even fixing this bug. At this point I've sank enough hours into this that I'll just give up (I need to work on some other things), and leave this for the next round of flake hunting.

For some reason, .focus() sometimes does not take effect, and the element immediately loses the focus again. This causes set_input_text() to select the whole page text instead of the input element, and failing to set the intended value.

martinpitt · 2023-10-12T05:13:50Z

We have to do something about this. This still breaks upstream cockpit-podman runs in podman way too often and creates noise, false negatives, and frustration.

martinpitt · 2023-10-12T06:02:01Z

Okay, so this still reproduces very well on our infrastructure, it's especially pronounced on /devel. Curiously, it passed with firefox, but TF runs firefox.

It sometimes reproduces locally with test/common/run-tests -j4 TestApplication.testPods{,2,3,4,5,6,7,8,9,10}. I cannot repro it on a bot when running the test in a loop. 🤯

Continuing from #1324 (comment) , it is still true that there are no podman events during this operation, although of course the log ordering is not very reliable. This also shows when ContainerHeader() gets-re-rendered; that does not happen in between the focus and key input, but it does get re-rendered again afterwards with the old value:

 log: XXX render ContainerHeader user admin twoOwners true ownerFilter all textFilter test-pod-1-system
<- {'type': 'undefined'}
-> wait: ph_is_present("#containers-filter:not([disabled]):not([aria-disabled=true])")
<- {'type': 'undefined'}
-> wait: ph_is_visible("#containers-filter:not([disabled]):not([aria-disabled=true])")
<- {'type': 'undefined'}
-> ph_focus("#containers-filter")
<- {'type': 'undefined'}
-> Input.dispatchKeyEvent({"type": "keyDown", "modifiers": 2, "text": "a", "windowsVirtualKeyCode": 65})
<- {}
-> Input.dispatchKeyEvent({"type": "keyUp", "modifiers": 2, "text": "a", "windowsVirtualKeyCode": 65})
<- {}
-> Input.dispatchKeyEvent({"type": "keyDown", "modifiers": 0, "text": "\b", "windowsVirtualKeyCode": 8})
<- {}
-> Input.dispatchKeyEvent({"type": "keyUp", "modifiers": 0, "text": "\b", "windowsVirtualKeyCode": 8})
<- {}
-> wait: ph_is_present("#containers-filter:not([disabled]):not([aria-disabled=true])")
> debug: podman system call 47 result: "[]\n"
<- {'type': 'undefined'}
-> wait: ph_is_visible("#containers-filter:not([disabled]):not([aria-disabled=true])")
> debug: podman system call 50 error: {"status":404,"reason":"Not Found","message":"Not Found","problem":null} content "{\"cause\":\"no such container\",\"message\":\"no container with name or ID \\\"96bd46d7e2110e6dc938c1086d93a292f02652b8b43de68c25d30dee54762143\\\" found: no such container\",\"response\":404}\n"
> debug: podman system call 48 result: "[]\n"
> log: 
> debug: podman system call 51: {"method":"GET","path":"/v1.12/libpod/containers/96bd46d7e2110e6dc938c1086d93a292f02652b8b43de68c25d30dee54762143/json","body":"","params":{"size":false}}
> debug: podman system call 49 result: "[]\n"
<- {'type': 'undefined'}
-> ph_blur("#containers-filter")
<- {'type': 'undefined'}
-> wait: ph_is_present("#containers-filter")
> debug: podman system call 46 result: "{\"RemovedCtrs\":{\"96bd46d7e2110e6dc938c1086d93a292f02652b8b43de68c25d30dee54762143\":null},\"Err\":null,\"Id\":\"22a53193294a0031c001f8ae19d260662d7e5357a5dc9cc4ab56a8158c54412d\"}\n"
<- {'type': 'undefined'}
-> wait: ph_is_visible("#containers-filter")
<- {'type': 'undefined'}
-> wait: ph_has_val("#containers-filter","")
> debug: podman system call 51 error: {"status":404,"reason":"Not Found","message":"Not Found","problem":null} content "{\"cause\":\"no such container\",\"message\":\"no container with name or ID \\\"96bd46d7e2110e6dc938c1086d93a292f02652b8b43de68c25d30dee54762143\\\" found: no such container\",\"response\":404}\n"
> log: 
> debug: podman system monitor {"status":"remove","id":"22a53193294a0031c001f8ae19d260662d7e5357a5dc9cc4ab56a8158c54412d","Type":"pod","Action":"remove","Actor":{"ID":"22a53193294a0031c001f8ae19d260662d7e5357a5dc9cc4ab56a8158c54412d","Attributes":{"containerExitCode":"0","image":"","name":"pod-1","podId":""}},"scope":"local","time":1697093311,"timeNano":1697093311385143777}
> log: XXX render ContainerHeader user admin twoOwners true ownerFilter all textFilter test-pod-1-system

With the confirm focus patch the test fails on that then:

<- {'type': 'undefined'}
-> wait: ph_is_present("#containers-filter:not([disabled]):not([aria-disabled=true])")
<- {'type': 'undefined'}
-> wait: ph_is_visible("#containers-filter:not([disabled]):not([aria-disabled=true])")
<- {'type': 'undefined'}
-> ph_focus("#containers-filter")
<- {'type': 'undefined'}
-> wait: document.activeElement == document.querySelector("#containers-filter")
[...]
wait_js_cond(document.activeElement == document.querySelector("#containers-filter")): Uncaught (in promise) Error: condition did not become true

So indeed the ph_focus() went into Nirvana.

martinpitt · 2023-10-12T07:26:19Z

Pushing the original hack again. There is absoeffingly nothing to grab on here, and locally this has survived

for i in `seq 5`; do test/common/run-tests -j4 TestApplication.testPods{,2,3,4,5,6,7,8,9,10} || break; done

Let's see whether CI agrees.

martinpitt · 2023-10-12T07:54:06Z

OK, it's stable now with the 10x amplified test in commit 0ce1e25 . So really, this is it from me -- I am not going to look at this minefield again. If someone else has an idea, please have at it 😁

jelly

I'm a bit reluctant to approve this, however the podman reverse dependency tests failing are also getting a bit on my nerves.

Wondering if we should make an issue to debug this further? Can it be a re-draw? Update in between which loses focus of the input?

martinpitt · 2023-10-12T08:53:14Z

@jelly See above, I logged re-renders, and didn't find anything. I have no ideas any more how to debug this..

martinpitt · 2023-10-12T08:55:24Z

@jelly: I created issue #1450 for it

martinpitt added the flake unstable test label Jul 14, 2023

martinpitt requested review from KKoukiou and marusak July 14, 2023 15:41

martinpitt mentioned this pull request Jul 14, 2023

Debug flakes #1324

Closed

martinpitt closed this Jul 17, 2023

martinpitt deleted the testpods-hack branch July 17, 2023 15:28

martinpitt restored the testpods-hack branch October 12, 2023 05:12

martinpitt reopened this Oct 12, 2023

martinpitt force-pushed the testpods-hack branch from e543081 to 852fe88 Compare October 12, 2023 05:17

martinpitt marked this pull request as draft October 12, 2023 05:17

martinpitt force-pushed the testpods-hack branch 2 times, most recently from 4f42f80 to 0ce1e25 Compare October 12, 2023 07:25

martinpitt removed request for marusak and KKoukiou October 12, 2023 07:25

martinpitt force-pushed the testpods-hack branch from 0ce1e25 to 91cbc7a Compare October 12, 2023 07:54

martinpitt marked this pull request as ready for review October 12, 2023 07:54

martinpitt requested review from jelly and marusak October 12, 2023 07:54

jelly approved these changes Oct 12, 2023

View reviewed changes

martinpitt mentioned this pull request Oct 12, 2023

#containers-filter loses focus in testPods #1450

Open

jelly merged commit d95b962 into cockpit-project:main Oct 12, 2023
18 checks passed

martinpitt deleted the testpods-hack branch October 12, 2023 09:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Add hack to avoid losing focus on #containers-filter #1356

test: Add hack to avoid losing focus on #containers-filter #1356

martinpitt commented Jul 14, 2023

marusak commented Jul 17, 2023

martinpitt commented Jul 17, 2023

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023 •

edited

Loading

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023

jelly left a comment

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023

test: Add hack to avoid losing focus on #containers-filter #1356

test: Add hack to avoid losing focus on #containers-filter #1356

Conversation

martinpitt commented Jul 14, 2023

marusak commented Jul 17, 2023

martinpitt commented Jul 17, 2023

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023 • edited Loading

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023

jelly left a comment

Choose a reason for hiding this comment

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023

martinpitt commented Oct 12, 2023 •

edited

Loading